Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #22: Add support for CKEditor 5. #24

Open
wants to merge 1 commit into
base: 1.x-1.x
Choose a base branch
from

Conversation

quicksketch
Copy link
Member

@quicksketch quicksketch commented Dec 6, 2024

Fixes #22. This also removes some support that is not applicable to Backdrop, including:

  • FCKEditor(!?) support
  • WYSIWYG module (from D7, no Backdrop version) support
  • CKEditor 4 (from D7, not the core Backdrop version) support

@nattywebdev
Copy link

nattywebdev commented Dec 19, 2024

Many thanks @quicksketch - I've tested this PR and can confirm it works with CKEditor 5.
The icon and file link are in separate <p> tags but I can live with that - a little css sorts it out. This is the code that was added to the body for me:

<p>
  <img class="file-icon" src="/core/modules/file/icons/application-pdf.png" alt="" title="application/pdf">
</p>
<p>
  <a href="/files/members/Face_masks_order_form_2020.pdf" title="Face masks order form 2020"><span class="file">Face masks order form 2020</span></a>
</p>

@quicksketch
Copy link
Member Author

@nattywebdev Thanks for testing! Do you mean that every time you insert a file, it creates a new paragraph tag? It should respect where the cursor is in the document, but I admit I did not test that level. It's entirely likely the the code I used will always wrap the newly inserted content in <p>.

@nattywebdev
Copy link

Revisiting this after the break and I find that the additional

tags are now not inserted in my example. No idea what caused that issue before but my current example works just fine:

<p><span class="file"><img alt="" class="file-icon" src="/core/modules/file/icons/application-pdf.png" title="application/pdf"> <a href="/files/docs/Test_Document_1.pdf" title="Test document" class="ext" target="_blank">Test document</a></span> </p>

So thank you @quicksketch !

@quicksketch
Copy link
Member Author

That's great, thank you @nattywebdev. However, now that backdrop/backdrop-issues#6770 has been released in 1.29.3, we can improve this code to use the new variable provided by CKEditor 5 core, so we don't have to track the instance ourselves. This can be merged as-is, but it could be improved by now removing the manual tracking.

@nattywebdev
Copy link

@quicksketch is this now dealt with by backdrop/backdrop-issues#6770 ? Or is something needed in the Insert module to access the relevant variable? I have no clue how to do that so will need help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Insert appears not to work with CKEditor 5
2 participants